Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel: rewrite MD5 .note.gnu.build-id with truncated git SHA1. #767

Merged
merged 9 commits into from
Apr 17, 2017

Conversation

htuch
Copy link
Member

@htuch htuch commented Apr 14, 2017

This is a workaround for
bazelbuild/bazel#2805.

Small reorganization of version_generated.cc rules, since we rely on them
here and this can also assist with the Google import of Envoy and its
versioning scheme.

Also added docs on build types and how to do release builds.

htuch added 2 commits April 14, 2017 10:09
This is a workaround for
bazelbuild/bazel#2805.

Small reorganization of version_generated.cc rules, since we rely on them
here and this can also assist with the Google import of Envoy and its
versioning scheme.

Also added docs on build types and how to do release builds.
bazel/README.md Outdated
modes](https://bazel.build/versions/master/docs/bazel-user-manual.html#flag--compilation_mode)
that Bazel supports:

* `fastbuild`: `-O0 -DDEBUG`, aimed at developer speed (default).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per other PR, let's just kill -DDEBUG after checking we don't use it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

bazel/README.md Outdated
that Bazel supports:

* `fastbuild`: `-O0 -DDEBUG`, aimed at developer speed (default).
* `opt`: `-O2`, for production builds and performance benchmarking.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to #720, but I definitely at least want the ability to set -ggdb3 on opt builds. (IMO there is no reason to not generate debugging info and have easy access when looking at core dumps, but to each there on). If we have an option builders can choose.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me add this there and here (I should have split out this doco into a separate PR, but oh well).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added --define debug_symbols=yes support. I actually think it probably would be just as valid to have just suggested that the invoker provider --copt=-ggdb3 as an extra bazel build arg, but this works too.

import sys

# This is what the part of .note.gnu.build-id prior to the MD5 hash looks like.
EXPECTED_BUILD_ID_NOTE_PREFIX = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your bazel hacking skills continue to astound me. 😉

achieved with:

```
bazel --bazelrc=/dev/null build -c opt //source/exe:envoy-static.stripped.stamped
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read correctly below, //source/exe:envoy-static.stamped is valid, right? (If I want a stamped, but not stripped build)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.

@mattklein123
Copy link
Member

@htuch sorry needs master merge again

@mattklein123 mattklein123 merged commit a645008 into envoyproxy:master Apr 17, 2017
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants